-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Retry LB listener methods after timeout #8630
Conversation
@@ -493,6 +493,14 @@ func resourceAwsLbListenerCreate(d *schema.ResourceData, meta interface{}) error | |||
return nil | |||
}) | |||
|
|||
if isResourceTimeoutError(err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the error is a resource.TimeoutError
and we hit this condition, its important that we reset the error object so the later error checking logic is not triggered. Otherwise as written this will work like the following:
resource.Retry
sets top levelerr
toresource.TimeoutError
isResourceTimeoutError(err)
conditional runsCreateListener
again (great!)- If
CreateListener
returns an error, it returns the error - If
CreateListener
doesn't return an error, theisResourceTimeoutError()
conditional exits, but the below error check (if err != nil
) will still have the originalresource.TimeoutError
error, so it will still return an error
The trick here is using a direct =
assignment in Go instead of :=
This allows us to simplify this logic like the following:
// ... new logic here ...
if isResourceTimeoutError(err) {
_, err = elbconn.CreateListener(params)
}
// ... existing error handling below here ...
if err != nil {
return fmt.Errorf("Error creating LB Listener: %s", err)
}
// ... existing happy path ...
You'll now get the outcome you're expecting both ways when resource.Retry
has a timeout error:
resource.Retry
sets top levelerr
toresource.TimeoutError
isResourceTimeoutError(err)
conditional runsCreateListener
again (great!)- If
CreateListener
returns an error, it resets the top level error object and the below error check (if err != nil
) will return that error - If
CreateListener
doesn't return an error, the top level error object is reset tonil
and the below error check (if err != nil
) will not trigger so the resource will continue on its merry way on the happy path
Hope this helps!
edit: Whoops I thought the original issue reference was the tech debt issue (#7873), but it might be good to reference that one as well. 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Fixes #8025
Release note for CHANGELOG:
Output from acceptance testing: